Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set GOMAXPROCS automatically #41

Merged
merged 1 commit into from
Dec 13, 2023
Merged

Conversation

mhmxs
Copy link
Contributor

@mhmxs mhmxs commented Dec 7, 2023

When the Go runtime starts it creates an OS thread for each CPU core. This means if you have a 16 core machine the Go runtime will create 16 OS threads - regardless of any CGroup CPU Limits. The Go runtime then uses these OS threads to schedule goroutines.

The problem is that the Go runtime is not aware of the CGroup CPU limits and will happily schedule goroutines on all 16 OS threads. This means that the Go runtime will expect to be able to use 16 seconds of CPU time every second.

Long stop the world durations arise from the Go runtime needing to stop Goroutine on threads that it’s waiting for the Linux Scheduler to schedule. These threads will not be scheduled once the container has used it’s CPU quota.

For more info please follow: https://www.riverphillips.dev/blog/go-cfs

Copy link
Contributor

@avestuk avestuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should set the integer CPU annotation on the VPA for this if we ever end up using a VPA.

https://github.com/kubernetes/autoscaler/blob/master/vertical-pod-autoscaler/README.md#using-cpu-management-with-static-policy

@luthermonson
Copy link
Collaborator

isnt there a threadiness for kubebuilder too? i forget out it works or is set but maybe we need to worry about that too? this is fine to merge as is tho imho

@mhmxs
Copy link
Contributor Author

mhmxs commented Dec 10, 2023

isnt there a threadiness for kubebuilder too? i forget out it works or is set but maybe we need to worry about that too? this is fine to merge as is tho imho

Please let me know if there is a better way. We don't have to worries too much about this, but setting the right number of CPUs overall makes sense.

@avestuk
Copy link
Contributor

avestuk commented Dec 11, 2023

isnt there a threadiness for kubebuilder too? i forget out it works or is set but maybe we need to worry about that too? this is fine to merge as is tho imho

So you can set the MaxConcurrentReconciles (defaults to 1) that controls how many go routines get created on the controller.

I don't think we have to do adjust that as part of this PR but I think it's good to note that it's currently defaulting to 1 and we'd probably want to revisit that.

@luthermonson luthermonson merged commit 8aaa696 into linode:main Dec 13, 2023
2 checks passed
@mhmxs mhmxs deleted the automaxprocs branch December 14, 2023 08:17
amold1 pushed a commit that referenced this pull request May 17, 2024
Co-authored-by: Richard Kovacs <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants